Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable list of commands from playground repositories #897 - Commands Blacklisted #23

Merged
merged 17 commits into from
Oct 6, 2024

Conversation

yashbudhia
Copy link
Contributor

Based on the issue Disable list of commands from playground repositories #897 in the main dice repository , this PR adds a

    1. Blacklist.go file which contains a list of blacklist commands and logic for blacklisting.
    1. Updated httpServer.go: Implement the service layer and reject with a forbidden status
    1. Also adds proper comments in the httpServer.go file for easier code readability

Tests -

Manually tested via postman for get as well as post routes and all blacklisted commands are throwing
errors in this fashion{"error": "ERR unknown command '{Command_name}"}

eg-
image

Scope for improvement

You can also test scenarios like:

  • Sending a blacklisted command to see if it responds with the correct error.
  • Trying different HTTP methods (GET, POST) on various endpoints.

Copy link
Contributor

@lucifercr07 lucifercr07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yashbudhia thanks for contributing, please add tests for this.

"strings"
)

var blacklistedCommands = []string{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we keep it as map bool, so that checks would be faster?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that would be faster, let me apply the changes

}

// IsBlacklistedCommand checks if a command is blacklisted
func IsBlacklistedCommand(cmd string) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename this to BlockListedCommand

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@yashbudhia
Copy link
Contributor Author

ok

@lucifercr07
Copy link
Contributor

@yashbudhia please resolve conflicts once.

@yashbudhia
Copy link
Contributor Author

yashbudhia commented Oct 6, 2024

@yashbudhia please resolve conflicts once.

I have resolved the conflicts and changed the name of the function to BlockListedCommand
also i have now used a map[string]bool to make command checks faster,

Copy link
Contributor

@lucifercr07 lucifercr07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the build

internal/server/http.go Outdated Show resolved Hide resolved
@yashbudhia
Copy link
Contributor Author

Please fix the build

i have fixed the build and also removed the comments

Copy link
Contributor Author

@yashbudhia yashbudhia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

desired changes applied

@lucifercr07 lucifercr07 merged commit d373641 into DiceDB:master Oct 6, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants